Skip to content

Migrated Logging functional tests to Vitest+Playwright+MSW#1535

Open
carterworks wants to merge 12 commits into
migrate-integration/00-infrafrom
migrate-integration/03-logging
Open

Migrated Logging functional tests to Vitest+Playwright+MSW#1535
carterworks wants to merge 12 commits into
migrate-integration/00-infrafrom
migrate-integration/03-logging

Conversation

@carterworks

@carterworks carterworks commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Changed Packages

  • core
  • reactor-extension

Description

Migrates the Logging functional tests to the new Vitest+Playwright+MSW harness.

Related Issue

Part of the functional test → integration test migration. See packages/browser/test/FUNCTIONAL_MIGRATION_PLAN.md.

Motivation and Context

The existing TestCafe functional test suite is being migrated to Vitest+Playwright+MSW to enable faster, more reliable CI testing without a running server. This PR is part of a stacked series — each PR migrates one test file.

Functional tests replaced:

  • packages/browser/test/functional/specs/Logging/C2583.js
  • packages/browser/test/functional/specs/Logging/C2584.js
  • packages/browser/test/functional/specs/Logging/C2586.js
  • packages/browser/test/functional/specs/Logging/C532204.js

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which does not add functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have added a Changeset file with a consumer-facing description of my changes.
  • I have signed the Adobe Open Source CLA or I'm an Adobe employee.
  • I have made any necessary test changes and all tests pass.
  • I have run the Sandbox successfully.

Stack

  1. Added MSW handlers, mock fixtures, and Vitest+Playwright test harness for functional test migration #1532
  2. Migrated Install SDK functional tests to Vitest+Playwright+MSW #1533

@carterworks carterworks added the ignore-for-release Do not include this PR in release notes label Jun 12, 2026
@carterworks carterworks self-assigned this Jun 12, 2026
@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR migrates four Logging functional tests (C2583, C2584, C2586, C532204) from TestCafe to the new Vitest+Playwright+MSW harness, adding logging.spec.js as the single changed file.

  • C2583 / C2584 use the extended test fixture from extend.js, which handles alloy setup, teardown, MSW worker lifecycle, and cookie cleanup automatically; consoleSpy is wired via beforeEach/afterEach in each describe block.
  • C2586 opts out of the extended fixture with raw vitestTest to set alloy_debug=true in the URL before alloy loads; the spy and withTemporaryUrl wrapper are managed inline.
  • C532204 patches and restores the four console methods inside a try/finally and relies on the absence of a thrown error as the implicit assertion, matching the original TestCafe test's intent.

Confidence Score: 5/5

Safe to merge; the migration faithfully reproduces the original test scenarios with no new broken paths.

All four test cases are correctly migrated. The alloy fixture handles isolation for C2583/C2584/C532204; the raw vitestTest approach for C2586 is the right design choice for URL-before-configure ordering.

No files require special attention.

Important Files Changed

Filename Overview
packages/browser/test/integration/specs/Logging/logging.spec.js New integration spec migrating 4 functional tests (C2583, C2584, C2586, C532204) to Vitest+MSW. C2586 uses raw vitestTest to set the URL before alloy loads; alloyConfig includes debugEnabled: false in C2586, testing URL-param-wins-over-config rather than the URL-only scenario in the original test.

Sequence Diagram

sequenceDiagram
    participant BH as beforeEach
    participant FX as alloy fixture (auto)
    participant T as Test body
    participant AH as afterEach

    Note over BH,AH: C2583 / C2584 flow
    BH->>BH: "consoleSpy = vi.spyOn(console, info)"
    FX->>FX: cookie cleanup
    FX->>FX: setupBaseCode()
    FX->>FX: setupAlloy()
    T->>T: alloy configure / sendEvent / getLibraryInfo
    T->>T: expect(searchForLogMessage(consoleSpy, ...))
    FX->>FX: cleanAlloy()
    AH->>AH: consoleSpy.mockRestore()

    Note over BH,AH: C2586 flow (vitestTest - no extended fixture)
    T->>T: "consoleSpy = vi.spyOn(console, info)"
    T->>T: "withTemporaryUrl - applyUrl alloy_debug=true"
    T->>T: setupBaseCode()
    T->>T: setupAlloy()
    T->>T: alloy configure / getLibraryInfo
    T->>T: expect(searchForLogMessage(consoleSpy, ...))
    T->>T: cleanAlloy()
    T->>T: finally consoleSpy.mockRestore()
Loading

Reviews (2): Last reviewed commit: "test(integration): migrate logging funct..." | Re-trigger Greptile

Comment thread packages/browser/test/integration/specs/Logging/logging.spec.js Outdated
Comment thread packages/browser/test/integration/specs/Logging/logging.spec.js Outdated
@carterworks carterworks force-pushed the migrate-integration/02-context branch from 31ef5eb to 5914312 Compare June 12, 2026 17:31
@carterworks carterworks force-pushed the migrate-integration/03-logging branch from 19ba2a4 to f847fde Compare June 12, 2026 17:31
@changeset-bot

changeset-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 30cac6a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

This was referenced Jun 12, 2026
@carterworks carterworks force-pushed the migrate-integration/02-context branch from 5914312 to f38d93a Compare June 12, 2026 17:55
@carterworks carterworks force-pushed the migrate-integration/03-logging branch from f847fde to c698d64 Compare June 12, 2026 17:55

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@carterworks carterworks changed the base branch from migrate-integration/02-context to migrate-integration/00-infra June 12, 2026 17:57

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

carterworks has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@carterworks carterworks force-pushed the migrate-integration/03-logging branch 4 times, most recently from 6242b77 to f15d638 Compare June 12, 2026 21:20
@carterworks carterworks marked this pull request as draft June 15, 2026 16:38
@carterworks carterworks force-pushed the migrate-integration/03-logging branch from f15d638 to f352c78 Compare June 15, 2026 19:59
@carterworks carterworks marked this pull request as ready for review June 16, 2026 15:11
carterworks and others added 11 commits June 26, 2026 14:17
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The C532204 test re-spied console.info, which the file's beforeEach had
already spied. vi.spyOn returns the existing spy, so binding `through`
to console.info bound it to that same spy and forwarding looped back
through the mock implementation (stack overflow). Forward to console
methods captured before beforeEach installs the spy instead.

Also snapshot the call count inside the try block: mockRestore in the
finally clears mock.calls, so reading it afterward always saw 0.
The console spies forwarded to the real console by default, so alloy's
debug logging spammed the test reporter. Stub them with a no-op
implementation: searchForLogMessage reads spy.mock.calls, not actual
output, so assertions are unaffected. This also drops the originalConsole
forwarding in C532204 (and with it the recursion risk it guarded against).
@carterworks carterworks force-pushed the migrate-integration/03-logging branch from 3471d09 to 1a47ae5 Compare June 26, 2026 20:17
Keep the original testcafe functional specs alongside the new
Vitest+Playwright+MSW integration suite until these migration branches
merge, so reviewers retain the pre-migration signal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignore-for-release Do not include this PR in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant